Avoid module-level imports of oscrypt #36
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Let me take this opportunity to thank you for important work on many of your projects, some of which have become a corner stone of the OST community. Your efforts are very appreciated.
Today I'd like to discuss the
oscrypto
dependency. As you are aware, there are issues in some of your projects which depend on minikerberos (skelsec/pypykatz#136, skelsec/msldap#37) due to a bad regex ifopenssl
has a double digit patch number. Currently, openssl-3.0.10 is the current version on Kali and Debian unstable and probably more. This is discussed here: wbond/oscrypto#78I'm not sure why exactly the
oscrypto
dependency is needed, ascryptography
is already a dependency due to theasysocks
dependency. I think it could be replaced bycryptography
, but unfortunately I lack the skills, time and motivation to do this.To alleviate the issue, I propose to avoid the module-level imports of
oscrypto
and move them to the functions where they are needed by merging this PR. This would allow us to use projects like LdapRelayScan which needminikerberos
, but not thepkinit
module. Note that minikerberos has over 900 dependents (including forks, not sure how accurate this information is though), even though only 26 files (also including forks and identical files) actually importminikerberos.pkinit
.Still, if you manage to remove the
oscrypto
dependency, I'd count that as a win. What do you think?The original commit message follows. Please note that I could not yet test this change.
Many dependents of minikerberos don't need
PKINIT
, so it makes sense to importoscrypto
only when needed. Especially becauseoscrypto<=1.3.0
does not work whenopenssl>=3.0.10
.See: wbond/oscrypto#78